Skip to content

fix(codex): steer active turns for /ps guidance - #1665

Open
AaronZ345 wants to merge 7 commits into
chenhg5:mainfrom
AaronZ345:agent/fix-codex-ps-steer-20260810
Open

fix(codex): steer active turns for /ps guidance#1665
AaronZ345 wants to merge 7 commits into
chenhg5:mainfrom
AaronZ345:agent/fix-codex-ps-steer-20260810

Conversation

@AaronZ345

Copy link
Copy Markdown
Contributor

Summary

  • steer an active Codex app-server turn with turn/steer instead of starting a second turn
  • preserve the active turn ID, buffered text, and prompt preamble while applying /ps guidance
  • keep the existing turn/start behavior for idle sessions and reject mismatched steering responses
  • make four pre-existing environment-sensitive tests hermetic so the full local suite is reliable

Why

/ps calls AgentSession.Send while the current turn is still running. The Codex app-server implementation previously treated every Send as turn/start, reset the active-turn state, and cleared buffered messages. That race could emit an empty result even though Codex later completed a full answer.

Codex exposes turn/steer for same-turn guidance. This change uses it with the active expectedTurnId, matching Codex's native guidance behavior without turning the guidance into a separate user turn.

Testing

  • go test ./...
  • go test -race ./agent/codex ./core ./platform/cloud-web
  • go vet ./...
  • go build ./...
  • generated the Codex 0.144.1 app-server JSON schema and verified the turn/steer request and response fields

Supersedes #1530.

@chenhg5 chenhg5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论: Approve

总体判断: 一个干净的 protocol-level fix——把 /ps guidance 从「启动新 turn」改成「steer 现有 turn」,复用 Codex 0.144.1 的 turn/steer JSON-RPC。架构变化很合理,race fix 正确,测试覆盖三个独立分支。建议合入。

Review 范围:

  • 看了 agent/codex/appserver_session.go 中 Send 的 refactor(activeTurn 捕获 + preamble 守卫修改 + 新增 steerTurn 函数)。
  • 看了 turnSteerResponse 类型 + steerTurn 的 turnId 校验逻辑。
  • 看了新增 3 个测试:SendSteersActiveTurn / SendStartsIdleTurn / SendRejectsMismatchedSteeringTurn
  • 看了 hermetic test fixes(launchd_test.go / gateway_test.go / poll_test.go / cuj_test.go / engine_test.go)—— 全部为环境隔离(t.Setenv("HOME", tempDir) 等),无逻辑改动。
  • CI: run 31399427489 全绿(lint 1m57s / unit-test 4m25s / smoke 26s / regression 27s / performance 57s)。

✅ 做得好的地方:

  • Race 修得精准:原 bug 是 Sendturn/start 调用时把 currentTurn 重置 + buffered messages 清空,导致 race 下 Send 后异步 turn 完整跑完但 cc-connect 已经丢弃了 state,结果是「/ps 触发了,但响应空」。修复后 steer 不重置 turn state,自然消除 race。
  • Preamble 守卫修正顺手做了if activeTurn == "" && !s.preambleSent 同时修了「/ps 重复 prepend preamble」的问题——preamble 本意是「首次 turn 提示 AI 工作模式」,重复 prepend 既浪费 token 也干扰 steering 语义。这条 fix 不在 race 范围内但与 steer 行为耦合(preamble 重复会让 /ps 看起来「没生效」),所以作者一起做是合理的。
  • TurnID 校验两段式:steer response 必须 turnId 非空 + 必须等于 expectedTurnID。这条防御了 Codex 未来 schema 变化时「返回错误 turnId 导致错误引导」的 silent failure。
  • Hermetic test fixes:4 个 test 文件的环境隔离改动(t.Setenv("HOME", ...) 等)让本地 go test ./... 全过不再依赖 launchd / real env。这是「顺手修一个无关 race」——做法很对,因为这次改 codex 不应该让本地其他包的测试跟着 flaky。
  • Supersedes #1530:作者明确标注了取代 #1530,避免 reviewer 困惑「为什么不用之前的尝试」。

🚨/🔴 必须处理:

  • 未发现。

🟠 建议改进(不阻塞):

  • Hermetic test fixes 的独立性launchd_test.go / gateway_test.go / poll_test.go / cuj_test.go 的改动其实是独立的 flaky-test 修复,但被绑在一个 codex PR 里。建议 merge 后另开一个 follow-up PR 把这些 fix 单独 cherry-pick 到 main(让 main 上的 go test ./... 立即稳定,不必等 codex PR 合并)。低优先
  • steerTurn 的 input 格式假设:当前实现把 input []map[string]any 直接序列化(与 startTurn 保持一致)。如果 Codex 0.144+ 的 steer 与 start 对 input schema 略有不同(例如 steer 不接受某些字段),会导致运行时错。建议作者加一个 _ = schema 注释或测试覆盖 input schema 兼容性。
  • currentTurn 状态管理:steer 后 currentTurn 是否更新?看 diff 没动。如果 Codex 在 steer 后会发出新的 turnId(同一 thread 多次 steer),cc-connect 应该跟新——否则后续 /ps 仍 steer 旧 turn。建议 author 加一个测试覆盖「连续两次 /ps」的 currentTurn 更新行为。
  • Pre-merge 真实环境验证:Codex 0.144.1 升级到更新版本时 turn/steer schema 可能变化。建议作者在 post-merge 留意 Codex release notes。

🔵 可选优化:

  • docs/superpowers/plans/2026-07-11-codex-ps-turn-steer.md 177 行 implementation plan——这种 plan 文件建议放仓库外(个人 PR-author 笔记),但仓库内也 OK,作为「为什么这样设计」的历史记录有价值。保留
  • expectedTurnID mismatch 时返回的具体错误信息 codex app-server turn/steer returned turn id %q, want %q——对 debug 友好。

Testing / Risk:

  • 已看到的验证: 3 个新测试覆盖三个分支(active / idle / mismatched);CI 全绿;go test -race 通过。
  • 未覆盖风险: 真实 Codex 0.144.1 跑多次 /ps 时 turnId 行为是否一致(作者没贴 raw wire log)。建议 post-merge 用真实 Codex 0.144.1 跑 10 次连续 /ps 看 steer 是否都成功。
  • Blast radius: 仅 agent/codex/appserver_session.go,不影响其他 agent。

Next step:

  • 建议 owner 直接 merge。Scope 小、架构对、测试全,race 修得精准。
  • post-merge 验证: 用飞书连一个 Codex 0.144.1+ session,发 5+ 次连续 /ps,确认每次 steer 都生效、buffered text 不丢失、最终响应完整。

@AaronZ345
AaronZ345 force-pushed the agent/fix-codex-ps-steer-20260810 branch 4 times, most recently from 482b53b to 17b0075 Compare August 16, 2026 14:44
@AaronZ345
AaronZ345 force-pushed the agent/fix-codex-ps-steer-20260810 branch 2 times, most recently from 0303603 to 66c26f7 Compare August 17, 2026 15:08
@AaronZ345
AaronZ345 force-pushed the agent/fix-codex-ps-steer-20260810 branch from 66c26f7 to 060cd56 Compare August 17, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants